Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix prefixed index rollover #3457

Merged
merged 3 commits into from
Dec 21, 2021
Merged

Fix prefixed index rollover #3457

merged 3 commits into from
Dec 21, 2021

Conversation

albertteoh
Copy link
Contributor

Signed-off-by: albertteoh see.kwang.teoh@gmail.com

Which problem is this PR solving?

Short description of the changes

  • Fixes the bug where read aliases are not rolled over, and remain on the first index.

Testing

  • Update unit tests.

  • Reproduced the bug locally; output of aliases after 3 rollovers with prefixes (note write aliases successfully rollover, but read aliases remain pointing to first index):

foo-jaeger-service-write foo-jaeger-service-000003 - - - -
foo-jaeger-span-read     foo-jaeger-span-000001    - - - -
foo-jaeger-service-read  foo-jaeger-service-000001 - - - -
foo-jaeger-span-write    foo-jaeger-span-000003    - - - -
  • After the fix, the aliases look like the following after 3 rollovers (note each rolled over index is tracked by the read alias):
foo-jaeger-service-read  foo-jaeger-service-000001 - - - -
foo-jaeger-span-read     foo-jaeger-span-000001    - - - -
foo-jaeger-span-read     foo-jaeger-span-000003    - - - -
foo-jaeger-span-write    foo-jaeger-span-000003    - - - -
foo-jaeger-service-read  foo-jaeger-service-000002 - - - -
foo-jaeger-service-read  foo-jaeger-service-000003 - - - -
foo-jaeger-service-write foo-jaeger-service-000003 - - - -
foo-jaeger-span-read     foo-jaeger-span-000002    - - - -
  • Checked counts of prefixed indices against prefixed alias:
GET /foo-jaeger-span-read/_search # 53
GET /foo-jaeger-span-000001/_search # 28
GET /foo-jaeger-span-000002/_search # 20
GET /foo-jaeger-span-000003/_search # 5
  • Checked aliases when no prefix is defined to ensure no regression introduced:
jaeger-span-read     jaeger-span-000002    - - - -
jaeger-service-read  jaeger-service-000003 - - - -
jaeger-service-write jaeger-service-000003 - - - -
jaeger-service-read  jaeger-service-000002 - - - -
jaeger-span-read     jaeger-span-000003    - - - -
jaeger-span-write    jaeger-span-000003    - - - -
jaeger-service-read  jaeger-service-000001 - - - -
jaeger-span-read     jaeger-span-000001    - - - -

@albertteoh
Fix prefixed index rollover
bd68e70 
Signed-off-by: albertteoh <see.kwang.teoh@gmail.com>
@albertteoh albertteoh requested a review from a team as a code owner December 17, 2021 13:57
@codecov
Copy link

codecov bot commented Dec 17, 2021 •
edited
Loading

Codecov Report

Merging #3457 (32c937f) into main (c456653) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3457      +/-   ##
==========================================
+ Coverage   96.45%   96.49%   +0.03%     
==========================================
  Files         262      262              
  Lines       15308    15311       +3     
==========================================
+ Hits        14766    14775       +9     
+ Misses        458      454       -4     
+ Partials       84       82       -2
Impacted Files Coverage Δ
cmd/es-rollover/app/flags.go 100.00% <100.00%> (ø)
cmd/es-rollover/app/index_options.go 100.00% <100.00%> (ø)
cmd/query/app/server.go 95.58% <0.00%> (+1.47%) ⬆️
cmd/query/app/static_handler.go 97.60% <0.00%> (+1.79%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c456653...32c937f. Read the comment docs.

@albertteoh
Simplify and fix tests
8bf3a49 
Signed-off-by: albertteoh <see.kwang.teoh@gmail.com>
@albertteoh albertteoh enabled auto-merge (squash) December 21, 2021 11:18
@albertteoh albertteoh merged commit 3a471a5 into jaegertracing:main Dec 21, 2021
@albertteoh albertteoh deleted the 3441-fix-rollover-prefix branch December 22, 2021 06:04
@pupseba pupseba mentioned this pull request Jan 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pavolloffay pavolloffay pavolloffay approved these changes

@joe-elliott joe-elliott Awaiting requested review from joe-elliott joe-elliott is a code owner automatically assigned from jaegertracing/jaeger-maintainers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Elasticsearch rollover doesn't assign read aliases correctly when used with a prefix
2 participants
@albertteoh @pavolloffay